Skip to content

Conversation

@Liam-DeVoe
Copy link
Contributor

A more permissive type hint here would be great so we can more correctly type HypothesisWorks/hypothesis#4265 🙂

@BoboTiG BoboTiG merged commit fbb0e21 into gorakhargosh:master Feb 10, 2025
21 checks passed
@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 10, 2025

I will be able to cut a new release, if you do not need more patch to be merged.

@Liam-DeVoe
Copy link
Contributor Author

wow, thanks for the fast response <3.

re: another patch - do you think that FileSystemEvent.{src_path, dest_path} could be easily changed to str instead of str | bytes? The usages I checked in watchdog all encode from bytes to str, though I might be missing a case where it doesn't.

I'm happy to submit a patch for this, unless you know for a fact it needs to be str | bytes because something needs to pass bytes to the event somewhere, in which case I'll handle it locally on our side.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 10, 2025

re: another patch - do you think that FileSystemEvent.{src_path, dest_path} could be easily changed to str instead of str | bytes? The usages I checked in watchdog all encode from bytes to str, though I might be missing a case where it doesn't.

I'm happy to submit a patch for this, unless you know for a fact it needs to be str | bytes because something needs to pass bytes to the event somewhere, in which case I'll handle it locally on our side.

Go ahead with a patch as it's not the first time I read this request 👍

@Liam-DeVoe
Copy link
Contributor Author

Hmm, it seems like we expect events to be created with byte paths iff we passed a byte path to Observer.schedule? In which case the type hint is correct. I don't think any action is needed here then.

No rush on cutting a release, BTW!

@Liam-DeVoe Liam-DeVoe deleted the schedule-type branch February 16, 2025 03:41
TobiasRzepka pushed a commit to TobiasRzepka/watchdog that referenced this pull request Apr 19, 2025
… the `pathlib.Path` support (gorakhargosh#1096)

Co-authored-by: Mickaël Schoentgen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants